Skip to content

Add support for many=True on create#151

Open
feelixe wants to merge 2 commits intobeda-software:masterfrom
feelixe:master
Open

Add support for many=True on create#151
feelixe wants to merge 2 commits intobeda-software:masterfrom
feelixe:master

Conversation

@feelixe
Copy link
Copy Markdown

@feelixe feelixe commented Oct 16, 2021

This adds support for initializing serializer with many=True. Works for create, not update.

The current exception when attempting many=True in the master branch is that initial_data is not set.
So basically, I've created a ListSerializer BaseNestedListSerializer. And assign this in BaseNestedModelSerializer in method many_init. The list serializer then iterates over self.get_initial() and sets initial_data on the child serializer. I've tried running this in my project and so far I've not experienced any problems.

I would love some help to review this and if there are some improvements that can be made and what steps need to be taken for a merge.

Example usage (Same as current usage, except if you want to use many=True, you have to instantiate the serializer with that kwarg):

serializers.py

from drf_writable_nested import WritableNestedModelSerializer

class MyModelSerializer(WritableNestedModelSerializer):
    owner = PersonSerializer() # Some nested serializer
    toys = ToySerializer(many=True) # Another nested serializer

    class Meta:
        model = MyModel
        fields = '__all__'

views.py

from rest_framework import viewsets
from .serializers import MyModelSerializer
from .models import MyModel

class MyModelViewset(viewsets.ModelViewSet):
    serializer_class = MyModelSerializer
    queryset = MyModel.objects.all()

    def get_serializer(self, *args, **kwargs):
        if self.action == 'create':
            kwargs['many'] = True
        return super().get_serializer(*args, **kwargs)

Or to allow for sending one or many:

from rest_framework import viewsets
from .serializers import MyModelSerializer
from .models import MyModel

class MyModelViewset(viewsets.ModelViewSet):
    serializer_class = MyModelSerializer
    queryset = MyModel.objects.all()

    def get_serializer(self, *args, **kwargs):
        if self.action == 'create' and isinstance(kwargs['data'], list):
            kwargs['many'] = True
        return super().get_serializer(*args, **kwargs)

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Oct 16, 2021

Codecov Report

❌ Patch coverage is 52.94118% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.18%. Comparing base (f7d470f) to head (9096e78).
⚠️ Report is 96 commits behind head on master.

Files with missing lines Patch % Lines
drf_writable_nested/mixins.py 52.94% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #151      +/-   ##
==========================================
- Coverage   98.16%   96.18%   -1.99%     
==========================================
  Files           3        3              
  Lines         218      262      +44     
==========================================
+ Hits          214      252      +38     
- Misses          4       10       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@robd003
Copy link
Copy Markdown

robd003 commented Jun 4, 2022

Any chance @ruscoder or @ir4y could look at this?

@wael-kad
Copy link
Copy Markdown

wael-kad commented Sep 2, 2022

Does the self.get_initial() in the create of BaseNestedListSerializer work if our main serializer has a PrimaryKeyRelatedField through a Foreign Key? 🤔

Copy link
Copy Markdown

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not also add the code examples as tests to verify the changes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants